Skip to content

transition in custom element #4998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

hontas
Copy link
Contributor

@hontas hontas commented Jun 10, 2020

fixes #1825

The problem

The style-element containing the transition CSS is appended to the head-element which is unseeable from within the shadow dom.

The fix

This fixes that by adding the style element to the shadow DOM (for custom elements).

Copy link
Member

@Conduitry Conduitry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to https://caniuse.com/#feat=mdn-api_node_getrootnode .getRootNode() is not supported on IE - and, since it's a method on an existing class, it's not something that can be polyfilled very well. I believe this change would break all transitions on IE.

@hontas
Copy link
Contributor Author

hontas commented Jun 11, 2020

@Conduitry so that's why nobody had thought of that "excellent" idea 🤦 😅
I'll be right back with an updated solution 👍

@Buk1m
Copy link

Buk1m commented Aug 31, 2020

This problem breaks svelte transitions whenever svelte component is injected into shadow dom. I'm not using custom-element option and transitions are still broken. I think it's a pretty common case to inject svelte components into shadow-dom. This PR would help my team a lot.

@brunoalano
Copy link

Is something blocking this PR to be merged after commit #5b5a934?

@MonkeyAndres
Copy link

As long as the PR is still open you can use the build script in the custom-elements template that I created. It fixes the transition using .getRootNode().

Custom element template: https://github.com/redradix/svelte-custom-element-template
Build script source: https://github.com/redradix/svelte-custom-element-template/blob/master/scripts/build.js

@Buk1m
Copy link

Buk1m commented Oct 28, 2020

Thanks @MonkeyAndres! Your solution is fixing all the most common problems with custom elements, but it's pretty hacky. I would advise anyone who want to use it, to read carefully through the build script. Just to be aware of what kind of hacks are there. Using regex to replace .ownerDocument with getRootNode() from build output is definitely not ideal.

@MonkeyAndres
Copy link

Totally agree with you @Buk1m as I say in the README this is my solution to these limitations, if you find out a less "hacky" way of fixing these problems feel free to open an issue or PR in the repo.
With these replace approach I was trying to change the less amount of code to make it work but yeah, isn't ideal 😅

@hontas
Copy link
Contributor Author

hontas commented Oct 29, 2020

@Conduitry could you please have a look at this PR again?

@hontas hontas force-pushed the custom-element-transition branch from a256c6e to f36e84c Compare October 29, 2020 09:50
@ScottAwesome
Copy link

Does anyone need help landing this? Really looking at svelte with custom elements but I’m really looking to help with getting PRs around it to land. I’m happy to help!

}

function get_root_node(node) {
if (is_function(node.getRootNode)) return node.getRootNode();
Copy link

@mahdimaiche mahdimaiche Feb 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a weird behaviour doing so, let's suppose we have a transition on hover on an element, if the element on which we want to apply the transition is directly a child of a shadow-root element it will be okay, if otherwise it's a child of a regular DOM node (let's say a div) it will keep on appending style tags on each hover. Looking recursively for a parenElement to get to the root of the webComponent solves the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, thanks. Could you show me your code so I can have a look as well?

@hontas hontas force-pushed the custom-element-transition branch 8 times, most recently from dce169b to 2b57dce Compare March 3, 2021 23:57
@hontas hontas force-pushed the custom-element-transition branch from 2b57dce to 894f8b7 Compare March 3, 2021 23:59
@vultix
Copy link

vultix commented May 25, 2021

@hontas Is there anything preventing this from getting merged?

@hontas
Copy link
Contributor Author

hontas commented May 27, 2021

@vultix yes I’m still waiting for feedback / approval from the maintainers on this one. I asked for another review a while back but I’m not sure how to escalate this to get attention.
@Conduitry could you please have another look on this one?

@hgiesel
Copy link
Contributor

hgiesel commented May 27, 2021

Isn't this PR a subset of the functionality of #5870?

@stale
Copy link

stale bot commented Dec 23, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale-bot label Dec 23, 2021
@baseballyama baseballyama added this to the 4.x milestone Feb 26, 2023
@dummdidumm dummdidumm modified the milestones: 4.x, one day Apr 11, 2023
@Rich-Harris
Copy link
Member

Apologies, looks like this got lost in the mix. I'm going to close it since Svelte 5 has a very different mechanism — happily, this should all Just Work since it's based on Web Animations rather than CSS hackery.

@Rich-Harris Rich-Harris closed this Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transitions in custom Elements